fix(relay): bound storage conformance admission time - #24
Draft
mfethe1 wants to merge 4 commits into
Draft
Conversation
Signed-off-by: Michael Feth <mfethe1@gmail.com>
Signed-off-by: Michael Feth <mfethe1@gmail.com>
Signed-off-by: Michael Feth <mfethe1@gmail.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A storage backend that stalls during the relay's conformance probe can leave startup waiting indefinitely. Bound the complete production probe to 120 seconds and fail startup when it expires. An unfinished conditional-write race is never admitted.
Select the regression tests in both unit-test runners and remove redundant ACP test invocations while preserving the existing full-crate lane.
Related issue
Related machine-home work: #15. Discovered while recovering the Mack relay.
Testing
Three real HTTP probe tests passed: a stalled backend was denied after 250 ms, a responsive backend passed, and a zero deadline failed before issuing a request. Removing the timeout made the regression fail; restoring it passed. The exact final head passed the relay binary-target check, formatting, and six runner routing/failure-propagation scenarios. Those runner scenarios use a captured subprocess stub; the subsequent full local CI also ran all 968 ACP tests successfully. Independent source review passed at
195c72764c65bf2a1ae3ff87576aca1874e3db3d.The actual native relay executable was tested with unchanged production settings of 32 writers, three rounds, and a 120-second deadline. Stalled storage caused exit 1 after 120.153 seconds from probe start, without admission or readiness. Responsive storage was admitted with zero transport drops, returned HTTP 200
{"status":"ready"}, and shut down cleanly. Root independently checked the actual binary hash, raw logs, and closed fixture ports. Two preliminary fixture setup/guard failures are retained; owned test databases, services, and private credentials were removed.Full
just cifailed only at its final mobile stage: 2,241 passed and eight golden comparisons failed. All eight actual images matched the independently verified unchanged-base images on Airy. All preceding stages passed, including 6,583 desktop tests. Fulljust testpassed its unit stages, then failed the Docker Compose infrastructure launcher withunknown shorthand flag: 'd' in -dand a service wait timeout. The isolated native startup checks passed separately.This change has not been deployed as part of this work. Hosted checks for this head have completed with image-build, Windows Rust, PostgreSQL, desktop smoke, and desktop integration failures. Hosted qualification and protected release approval remain outstanding; successful local checks do not qualify this PR for merge.